-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge my personal messages #62
Conversation
As this is already a breaking change, how do you think about moving the new |
} | ||
// TODO: Allow more than one variable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(just a small note: the comment here is actually for the message.fillname
part)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, whoops, i thought it was about what I implemented.
Anyway, if that's really needed, it should probably be an issue at this point, but I don't really see the need for multiple inputs as of right now.
That's true. I wasn't sure if the keys were used for anything significant anywhere, so I kept them as it was. Arisa is prepared now for this change, so it shouldn't be as breaking as it was anymore, it was a easier fix than I thought. Unless I've forgotten something else that relies on the helper messages, we should be good now. If we go for breaking changes, it might make sense to revamp the format of the file completely. For instance on many messages you might just want to specify "all" projects. That's currently not possible. That could be fixed by for instance leaving out the "projects" property if all projects should be specified. And |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some comments about sections which could possibly be improved.
Some have not been changed by this pull request, but if there will be such massive changes anyways, it might make sense to perform these smaller edits as well.
shortcut: crash | ||
message: |- | ||
*Thank you for your report!* | ||
However, this issue is {color:#FF5722}*Invalid*{color}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on purpose that this says the issue is invalid, while the message for bds, mcpe and realms does not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is. But I don't really want to tamper too much with the MCPE messages, I don't really know how they're used.
- mc | ||
- mcl | ||
name: Attach launcher log | ||
shortcut: llog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply log? There doesn't seem to be any message that currently uses log as shortcut.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though log
could be used for the game log in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The game log is included in "Attach crash report".
awaiting-response: | ||
- project: | ||
- bds | ||
- mc | ||
- mcl | ||
- mcd | ||
- mce | ||
- mcpe | ||
- mctest | ||
- realms | ||
- web | ||
name: Awaiting Response | ||
shortcut: ar | ||
message: |- | ||
%awaiting_response% | ||
|
||
%quick_links% | ||
fillname: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how this is intended to be used, but maybe it would make sense to add a variable to this one for the question being asked?
|
||
%quick_links% | ||
fillname: [] | ||
duplicate-of-mcl-5638: | ||
- project: mcl | ||
name: Duplicate of MCL-5638 | ||
shortcut: jre | ||
name: MCL-5638 / Unable to locate Java runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did like those being sorted in the box together (because all duplicate messages used to start with "Duplicate".
But tbh, the dropdown becomes more and more crowded anyways, making it harder to find stuff, and we should probably think about a better solution in general.
duplicate-resolved: | ||
- project: | ||
- bds | ||
- mc | ||
- mcapi | ||
- mcd | ||
- mce | ||
- mcl | ||
- mcpe | ||
- mctest | ||
- realms | ||
- web | ||
name: Duplicate (resolved) | ||
shortcut: rdup | ||
message: |- | ||
*Thank you for your report!* | ||
We're tracking this issue as *%s%*, so this ticket is being resolved and linked as a *duplicate*. | ||
|
||
That ticket has already been resolved. Please take a look at it to learn more. | ||
|
||
%search_for_dupes% | ||
|
||
%quick_links% | ||
fillname: | ||
- Enter parent issue ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need that message? We already have individual messages for fixed, tech support, wai and won't fix.
Other resolutions are:
- Incomplete, but we don't use Incomplete tickets as a parent (or otherwise reopen it)
- Cannot Reproduce, but that either means it's fixed, or it should be reopened
- AR - might make sense to add a message for that saying something like "... but the parent is currently missing vital information. Please do take a look and see if you can help out, in order to get this bug fixed as soon as possible"
- Done - is not used anymore afaik.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory we are missing one for "Invalid", though this message might not be that useful for this either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we have one for tech support. Account issues obv. don't get linked, and a duplicate feature request doesn't happen often, and tbh I don't bother searching for a parent in this case and just resolve as invalid.
Minecraft has several different editions, and some features vary from edition to edition. Often this happens when one edition evolves faster than another, and tweaks or balances are made that don't propagate immediately to other editions. | ||
Parity issues are not tracked on the bug tracker but should instead be reported on the [Feedback website|https://feedback.minecraft.net/hc/en-us/community/topics/360001191251-Parity]. | ||
*Thank you for your report!* | ||
However, this issue is {color:#FF5722}*Invalid*{color}. | ||
|
||
You have posted a parity issue. Minecraft has several different editions, and some features vary from edition to edition. | ||
This is especially the case with features that were added a long time ago, because one edition evolved faster than another, and tweaks or balances were made that did not propagate immediately to other editions. | ||
These differences between the different versions of the game are called "Parity issues". | ||
|
||
We only accept bug reports about a parity issue if it fulfills the following criteria: | ||
* The feature affected by the parity issue is present in both Bedrock Edition and Java Edition in the latest release or development version | ||
* The feature behaves differently in one edition than in the other | ||
* The parity issue was introduced in Buzzy Bees (Bedrock Edition 1.14 / Java Edition 1.15) or later and was not present before | ||
|
||
All other parity issues are not tracked on the bug tracker but should instead be reported on the [Feedback website|https://feedback.minecraft.net/hc/en-us/community/topics/360001191251-Parity]. | ||
|
||
For more information about this, please take a look at [this post|https://www.reddit.com/r/Mojira/comments/g9rjfh/from_now_on_well_accept_bug_reports_about_certain/] on our subreddit. | ||
|
||
%quick_links% | ||
fillname: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That message has been changed already, is this change still required / better than the one we already have?
name: Attach hserr_pid file | ||
shortcut: hserr | ||
message: |- | ||
Is there a file with a name starting with {{hs_err_pid}} in your {{[https://minecrafthopper.net/help/guides/finding-minecraft-data-folder/|.minecraft]}} folder? If so, please attach it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also tell the user to make their report private, see MC-207889
This has diverged too much from the main branch and I also believe that the current messages are okay. I might still pick one or two of these changes up later but that would be separate, smaller PRs. |
Since I plan on no longer using my personal database of messages, I added all messages that were not present in this repo yet and that I thought might be useful here.
Also includes some more improvements on existing messages. Please report back if you have any issues with what I changed.
Additionally, I have moved over some more commonly used things into variables, and compacted the existing variables down a bit. This also means that ⚠ this is a breaking change ⚠ since variables can include other variables now, which is also why I needed to adjust the JavaScript file a bit (recursion limit: 10). Other projects that rely on this may not expect that and might not be able to parse variables containing other variables correctly.
This PR is based on #61, so that should be merged before this one. I made a separate PR because this is a much larger change and is more likely to take longer to discuss.